Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[expo-sqlite] Fix Web support #8518

Merged
merged 9 commits into from May 29, 2020
Merged

[expo-sqlite] Fix Web support #8518

merged 9 commits into from May 29, 2020

Conversation

sjchmiela
Copy link
Contributor

Why

I wanted to clear the way for #8242 to be landed (or not). I noticed that web support doesn't look like it should work and that it isn't tested anywhere.

How

  • added SQLite test suite to set run on web
  • changed the way we fetch values in test suite from using ._array[0] to .item(0)
  • added explanations in SQLite.types.ts about why we may need a copy of @types/websql
  • added Window declaration to types declarations
  • removed @types/websql dependency as it's not even importable
  • fixed web counterpart — SQLite.web.ts — by fixing the method header and gracefully handling situation where openDatabase is not available (I think).

Test Plan

Test suite passes.

@sjchmiela sjchmiela marked this pull request as ready for review May 27, 2020 18:06
@sjchmiela sjchmiela requested a review from mczernek as a code owner May 27, 2020 18:06
@sjchmiela sjchmiela requested a review from EvanBacon May 27, 2020 18:07
Copy link
Contributor

@EvanBacon EvanBacon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good work!

@EvanBacon EvanBacon added Platform: web Using Expo in the browser SQLite labels May 28, 2020
@sjchmiela sjchmiela merged commit e11b798 into master May 29, 2020
@sjchmiela sjchmiela deleted the @sjchmiela/fix-sqlite-web branch May 29, 2020 08:52
@github-actions
Copy link
Contributor

Native Component List for this branch is ready

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Platform: web Using Expo in the browser SQLite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants